Complete source images - #866
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
aranega
left a comment
aranega
left a comment
There was a problem hiding this comment.
The PR looks good to me, I just added a comment/nitpick about the fact of using some hardcoded path, but I'm unsure the line in question is something which is supposed to run on the CI/CD or on the user's machine.
| arguments = deployment_step.get("arguments") | ||
| if arguments: | ||
| # Apply the generated image overrides after values.yaml so editing them wins | ||
| values_overrides_file = f"./{DEPLOYMENT_PATH}/{HELM_CHART_PATH}/{VALUES_OVERRIDES_PATH}" |
There was a problem hiding this comment.
nitpick: it could be cool to use more Path object from the pathlib builtin libs as it removes the use of / as os dependent separator. This is something which is not entirely coherent through our base code (CH-269 points to this as something to solve), but in some cases, I'm not sure if that's meant to be run only on the CI/CD (usually linux-based) or on the user machine.
There was a problem hiding this comment.
It's a good argument to attempt to harmonize the way we handle paths, but as long as the code is not fully coherent it's a hard call to enforce it on a single PR. What about a PR that targets paths specifically and moves everything to pathlib?
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…to feature/CH-287
| include=['samples'], domain="my.local", namespace='test', local=False, | ||
| tag=1, secured=True) | ||
| helm_path = out_path / HELM_CHART_PATH | ||
| overrides = yaml.safe_load(open(helm_path / VALUES_OVERRIDES_PATH)) |
| create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES, str(solution)], output_path=out_path, | ||
| include=['myapp'], exclude=['events'], domain="my.local", | ||
| namespace='test', local=False, tag=1) | ||
| loaded = yaml.safe_load(open(out_path / HELM_CHART_PATH / VALUES_OVERRIDES_PATH)) |
| import subprocess | ||
|
|
||
| from cloudharness_utils.constants import VALUES_MANUAL_PATH, HELM_CHART_PATH | ||
| from cloudharness_utils.constants import VALUES_MANUAL_PATH, VALUES_OVERRIDES_PATH, HELM_CHART_PATH |
| from ch_cli_tools import configurationgenerator | ||
| from ch_cli_tools.configurationgenerator import * | ||
| from ch_cli_tools.helm import * | ||
| from ch_cli_tools.configurationgenerator import * |
2 participants
Closes CH-287
Implemented solution
All image paths/values are exported as values-overrides.yaml, so it's easier to change everything in one place.
harness-deploymentgenerate that file with every image that's relevantHow to test this PR
Run
harness-deploymentand then checkdeployment/helm/values-overrides.yaml.It should contain all the relevant source images.
Sanity checks:
Breaking changes (select one):
breaking-changeand the migration procedure is well described abovePossible deployment updates issues (select one):
alert:deploymentTest coverage (select one):
Documentation (select one):
Nice to have (if relevant):